-
-
Notifications
You must be signed in to change notification settings - Fork 14.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
swappy: Init at 1.2.1 #81116
swappy: Init at 1.2.1 #81116
Conversation
sha256 = "08r9hmhzzb5ac4g6zwm7w05n99v0hl2h0w4d8i694hg4pyjxb95y"; | ||
}; | ||
|
||
buildInputs = [ meson ninja pkgconfig cmake scdoc gio-sharp libnotify gtk pango cairo wayland]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So upstream said:
gio-sharp does not exist in Arch.
Are you sure gio-sharp
is absolutely needed? How come he hasn't said so?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I get errors without it, e.g.:
../src/pixbuf.c:3:10: fatal error: gio/gunixoutputstream.h: No such file or directory
3 | #include <gio/gunixoutputstream.h>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's part of glib2
on Arch. Somehow it compiles fine on Arch without specifying the lib explicitly. I just pushed a fix in the meson
build file.
buildInputs = [ meson ninja pkgconfig cmake scdoc gio-sharp libnotify gtk pango cairo wayland]; | ||
|
||
meta = { | ||
homepage = https://github.com/jtheoof/swappy; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
homepage = https://github.com/jtheoof/swappy; | |
homepage = "https://github.com/jtheoof/swappy"; |
}; | ||
|
||
buildInputs = [ meson ninja pkgconfig cmake scdoc gio-sharp libnotify gtk pango cairo wayland]; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As for jtheoof/swappy#10 , try to add (untested):
nativeBuildInputs = [ glibc.dev ];
Source: https://linux.die.net/man/3/clock_gettime + nix-locate unistd.h
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did not work:
../src/buffer.c: In function 'randname':
../src/buffer.c:12:3: error: implicit declaration of function 'clock_gettime' [-Werror=implicit-function-declaration]
12 | clock_gettime(CLOCK_REALTIME, &ts);
| ^~~~~~~~~~~~~
../src/buffer.c:12:17: error: 'CLOCK_REALTIME' undeclared (first use in this function)
12 | clock_gettime(CLOCK_REALTIME, &ts);
| ^~~~~~~~~~~~~~
../src/buffer.c:12:17: note: each undeclared identifier is reported only once for each function it appears in
../src/buffer.c: In function 'create_shm_file':
../src/buffer.c:45:7: error: implicit declaration of function 'ftruncate'; did you mean 'strncat'? [-Werror=implicit-function-declaration]
45 | if (ftruncate(fd, size) < 0) {
| ^~~~~~~~~
| strncat
cc1: all warnings being treated as errors
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be fixed now too.
The latest build from |
1.0.0 is out. https://github.com/jtheoof/swappy/releases @matthiasbeyer don't hesitate to say if you don't have the time for this PR anymore. One of us could pick it up. |
No, I'm fine. I pushed an update to 1.0.0, but it still fails to compile because of |
Can't really help you, I don't have nixOS installed. On Arch, |
nativeBuildInputs = [ glibc.dev ]; | ||
|
||
buildInputs = [ meson ninja pkgconfig cmake scdoc gio-sharp libnotify gtk pango cairo wayland]; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It builds with this change:
preConfigure = '' | |
sed -i "s#gio/gunixoutputstream.h#gio-unix-2.0/gio/gunixoutputstream.h#" src/pixbuf.c | |
''; | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or maybe there's a way to add ${glib.dev}/include/gio-unix-2.0
to the include path.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just use:
mesonFlags = [
# TODO: https://github.com/NixOS/nixpkgs/issues/36468
"-Dc_args=-I${glib.dev}/include/gio-unix-2.0"
];
and you can remove the nativeBuildInputs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and you can remove gio-sharp
.
There was a recent patch in swappy: jtheoof/swappy#42 that changes slightly how |
jtheoof/swappy@606ce38 has the same problem. |
@matthiasbeyer friendly ping |
Not sure what I should do here. |
try adding: mesonFlags = [
# TODO: https://github.com/NixOS/nixpkgs/issues/36468
"-Dc_args=-I${glib.dev}/include/gio-unix-2.0"
]; You can check the URL for more info. |
|
||
nativeBuildInputs = [ glibc.dev ]; | ||
|
||
buildInputs = [ meson ninja pkgconfig cmake scdoc gio-sharp libnotify gtk pango cairo wayland]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
build time only dependencies are declared in
nativeBuildInputs
https://nixos.org/nixpkgs/manual/#reviewing-contributions-new-packages
So I guess at least: meson, ninja, pkgconfig, cmake. Not sure about the others.
and it looks like "glibc.dev" can be removed;
@@ -2235,6 +2235,8 @@ in | |||
|
|||
bash-supergenpass = callPackage ../tools/security/bash-supergenpass { }; | |||
|
|||
swappy = callPackage ../applications/misc/swappy { gtk = gtk3; }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
flameshot, a similar tool is in pkgs/tools/misc/flameshot. Not sure if it's important.
|
||
meta = { | ||
homepage = "https://github.com/jtheoof/swappy"; | ||
description = "A Wayland native snapshot editing tool, inspired by Snappy on macOS "; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe remove the whitespace at the end
it seems I was able to build using: nativeBuildInputs = [ glib meson ninja pkgconfig cmake scdoc ];
buildInputs = [ cairo pango gtk libnotify wayland glib ];
strictDeps = true; Not sure why I need |
@matthiasbeyer friendly ping |
With your suggested change:
|
@GrahamcOfBorg build swappy |
Reviewed points
Possible improvementsCommentsOther reviewers might want to double check |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
Just noticed v1.2.1 was cut 14 days ago. Not meaning to block anything. |
@matthiasbeyer friendly ping |
5120e7a
to
e943f90
Compare
since this is the first time it's been packaged, I think it should be a single commit. I don't see much value in having 5 commits. |
@matthiasbeyer friendly ping. Can we get this in the release please? |
Sure, what's missing from your point of view? |
I already approved but maybe Jon's meta suggestion and squash the commits and it will be good to go. |
Signed-off-by: Matthias Beyer <mail@beyermatthias.de>
e943f90
to
a04e96d
Compare
@jonringer can you merge this please? |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
Friendly ping @jonringer since two weeks have passed since the last one. I'm looking forward to using this new tool in my workflow. It would also be great if this package was backported to the 20.09 release (unless the window is closed?). |
@marsam help merge this please. Jon must be pretty busy these days. |
Thank you very much 😄 |
WIP because waiting for
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)